Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Knx interfacer #200

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

lo92fr
Copy link

@lo92fr lo92fr commented Sep 13, 2023

Add a new interfacer to connect to a KNX bus to collect datagram from KNX group.

@glynhudson
Copy link
Member

Thanks,

Is it possible to add a readme with some documentation and link it into the docs here: https://github.com/openenergymonitor/emonhub/blob/master/docs/emonhub-interfacers.md#list-of-interfacers---links-to-github

@lo92fr
Copy link
Author

lo92fr commented Sep 13, 2023

Hello,

Yes, sure.
Just add the documentation to the pull request, and also modify the page on th documentation.

Laurent

@lo92fr
Copy link
Author

lo92fr commented Dec 5, 2024

Hello guys,

This pull request is open for more then one year !
Can you tell me if somethings wrong, or if we can hope to get it merge in a near future.

Thanks,
Laurent.

Copy link
Contributor

@alexandrecuer alexandrecuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont have a KNX gateway to test :-) but very nice work first of all 👍

self.loop = asyncio.get_event_loop()

task = self.loop.create_task(self.initKnx(gateway_ip, local_ip))
self.loop.run_until_complete(task)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a throught, is it threadsafe to use run_until_complete like that ?

self.ser = False


def action(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is really needed.

self.buffer.storeItem(f)


async def waitSensor(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used somewhere ?

def set(self, **kwargs):
for key, setting in self._KNX_settings.items():
# Decide which setting value to use
if key in kwargs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do setting = kwargs.get(key, self._KNX_settings[key]) and remove the whole if else

metters = self._settings['meters']

self.sensor={}
for metter in metters:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible to use items() like you do below ?

for metter in metters:
dpPoint = metters[metter]

for dpKey in dpPoint:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same I think you can iterate with items()

use var name like dp_config (snake case) and not dpConfig (camel case). the OEM maintainers are not using pylint to analyse their code but I think they should :-) I think the xknx lib is following this pattern and I think emonhub began like that, see the emonhub_interfacer base code

import threading

from emonhub_interfacer import EmonHubInterfacer
from xknx import XKNX
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xknx should be integrated in the dependencies....or maybe just a mention in the interfacer README



class Updater(threading.Thread):
def __init__(self, knxIntf):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you dont give any var when you initialize the updater in self.start. What is knxIntf ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants